[ERP-2110 & ERP-2111] Replaced all remaining Alert and Text Components #30
Conversation
…s to our helper components
| return ( | ||
| <TextWithLink | ||
| textBeforeLink={"You may need to enable the "} | ||
| link={{ |
There was a problem hiding this comment.
The color is different than in the main branch and doesn't look quite right because the color is not unset, and "teal.500" is the default now. Is there a reason for that, or can we use "blue.500" here?
There was a problem hiding this comment.
Yes, we can use blue.500
| import InstructionHeading from "./components/InstructionHeading"; | ||
| import InstructionText from "./components/InstructionText"; | ||
| import { OperatingSystemTabs } from "./components/OperatingSystemTabs"; | ||
|
|
||
| function LyraInstructions({ username }) { | ||
| const enableSSHText = () => { |
There was a problem hiding this comment.
Rather than making this a function, can we update AlertHelper to support passing React elements as the value for alertMessage?
You can use React's isValidElement in the condition
There was a problem hiding this comment.
The component itself already supporting passing message prop as a function or simple string, we can just add a children prop to support TextWithLink component being wrapped by AlertHelper in this case.
… support for wrapping child elements for AlertHelper.
| @@ -46,9 +32,21 @@ function LyraInstructions({ username }) { | |||
| <Box> | |||
| <AlertHelper | |||
| alertType={"info"} | |||
| alertMsg={enableSSHText} | |||
| // alertMsg={enableSSHText} | |||
| @@ -5,6 +5,7 @@ export default function AlertHelper({ | |||
| alertType = "info", | |||
| alertMsg = null, | |||
| onClose = {}, | |||
| children = null, | |||
There was a problem hiding this comment.
I'm not really a fan of having two different props to set the same thing, as you have to know internal details of the component in order to know which prop has precedent. Is there a reason to not use one?
There was a problem hiding this comment.
Actually we can just make one props to support multiple types.
Replaced all remaining Alert and Text Components to our helper components